Reviewable status: 0 of 4 files reviewed, 1 unresolved discussion (waiting on @CydeWeys and @jianglai)

core/src/main/java/google/registry/beam/common/RegistryPipelineWorkerInitializer.java line 78 at r2 (raw file):

    // as you can build the entities.
    if (registryOptions.getUseSelfAllocatedId()) {
      IdService.useSelfAllocatedId();

Instead of using a flag, may be we can provide a bring-your-own-id-service api.

Open up IdService to accept user provided Supplier if the environment is BEAM,
and do not provide a default implementation (throw is on beam and supplier is not provided.).

Code quote:

    if (registryOptions.getUseSelfAllocatedId()) {
      IdService.useSelfAllocatedId();
    }

Reviewable status: 0 of 4 files reviewed, 1 unresolved discussion (waiting on @CydeWeys and @jianglai)

core/src/main/java/google/registry/beam/common/RegistryPipelineWorkerInitializer.java line 78 at r2 (raw file):

This way we can plug in a raw datastore api to allocate ids.

Reviewable status: 0 of 4 files reviewed, 2 unresolved discussions (waiting on @CydeWeys, @jianglai, and @weiminyu)

core/src/main/java/google/registry/model/IdService.java line 67 at r2 (raw file):

   * @see #isSelfAllocated
   */
  public static void useSelfAllocatedId() {

Can we use different verbiage for this? E.g. "fake id", "testing id", "fake testing id" or similar? I don't think "self-allocated ID" conveys the right meaning, namely, that this is horribly unsafe for anything in production.

PTAL.

Reviewed 4 of 4 files at r1, 2 of 2 files at r2, 3 of 3 files at r3, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @CydeWeys and @weiminyu)

core/src/main/java/google/registry/beam/common/RegistryPipelineWorkerInitializer.java line 78 at r2 (raw file):

Done.

core/src/main/java/google/registry/model/IdService.java line 67 at r2 (raw file):

Reworked the class to take a ID supplier per Weimin's suggestion. I think the supplier used in test can still be called SelfAllocatedIdSupplier as it is an accurate description of what it does, whereas a FakeIdSupplier does not convey exactly what kind of "fake" it is, e.g. is it a constant (which should probably better named ConstantIdSupplier), or random (RandomIdSupplier).

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @CydeWeys and @weiminyu)

core/src/main/java/google/registry/model/IdService.java line 48 at r3 (raw file):

Just to clarify, do you mean to have a lookup map from the Beam enums to ID suppliers that one use to determine the override?

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @CydeWeys and @weiminyu)

core/src/main/java/google/registry/model/IdService.java line 48 at r3 (raw file):

Also, I think the current iteration is explicit enough to discourage the use of ID supplier overrides. It also allows for the further migration to a SQL-provided ID (as another ID provider), by which time we can simply remove the setter, as Beam has access to SQL. It doesn't seem worth it to make the Beam flags more flexible as it would just be throwaway work.

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @weiminyu)

Reviewed 1 of 2 files at r5, 1 of 1 files at r6, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @weiminyu)